Skip to content

Use Nginx config with outlets #913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 7, 2025
Merged

Use Nginx config with outlets #913

merged 2 commits into from
May 7, 2025

Conversation

@nbianca
Copy link
Member

@nbianca nbianca commented Jan 15, 2025 •
edited
Loading

This config has several 'include' statements that serve as extension
outlets. Most of the "find & replace" instructions from templates
files have been replaced to use outlets.

This commit also deprecates IPv6 template in favour of an automatic
implementation that relies on container's connectivity.

@nbianca nbianca marked this pull request as draft January 17, 2025 16:12
@nbianca nbianca changed the title Add nginx config file and support for outlets Use new Nginx config with outlets Jan 17, 2025
@nbianca nbianca force-pushed the dev-nginx-conf branch 4 times, most recently from ca741a9 to 12299cc Compare January 22, 2025 19:33
@nbianca nbianca changed the title Use new Nginx config with outlets Use Nginx config with outlets Jan 22, 2025
@nbianca nbianca marked this pull request as ready for review January 22, 2025 19:34
tgxworld
tgxworld reviewed Jan 23, 2025
View reviewed changes
tgxworld
tgxworld reviewed Jan 24, 2025
View reviewed changes
@Firefishy
Copy link
Contributor

The nginx config used in the stable branch now has the nginx "outlets" includes.

As an outsider who has to maintain some custom pups for adding extra IPv6 and SSL support, the "outlets" seems like a reasonable way to clean things up. Nice work @nbianca 👍

1 minor suggestion: The since nginx includes are included in alphabetical order, it may then make sense to prefix numerically to enforce order eg: 100-name.conf , 200-something-else.conf.

nbianca added 2 commits May 6, 2025 22:48
@nbianca
Use Nginx config with outlets
900902f 
This config has several 'include' statements that serve as extension
outlets. Most of the "find & replace" instructions from templates
files have been replaced to use outlets.

This commit also deprecates IPv6 template in favour of an automatic
implementation that relies on container's connectivity.
@nbianca
Prefix outlets with a priority
ed762bd
@nbianca nbianca merged commit 7b042d6 into main May 7, 2025
5 checks passed
@nbianca nbianca deleted the dev-nginx-conf branch May 7, 2025 15:56
@MichaIng
Copy link
Contributor

MichaIng commented May 8, 2025 •
edited
Loading

Since the web.socketed template now aims to edit /etc/nginx/conf.d/outlets/server/10-https.conf and /etc/nginx/conf.d/outlets/server/20-https.conf both, while depending on web.ssl template only one of them exists, it fails. Either both files need to exist, or the socketed template needs to edit them only if existing.

tgxworld added a commit that referenced this pull request May 9, 2025
@tgxworld
Revert "Use Nginx config with outlets (#913)"
ab6db99 
This reverts commit 7b042d6.

`web.socketed.template.yml` is broken
tgxworld added a commit that referenced this pull request May 9, 2025
@tgxworld
Revert "Use Nginx config with outlets (#913)" (#958)
3c22348 
This reverts commit 7b042d6.

`web.socketed.template.yml` is broken
nbianca added a commit that referenced this pull request May 9, 2025
@nbianca
Reapply "Use Nginx config with outlets (#913)" (#958)
d5792cd 
This reverts commit 3c22348.
nbianca added a commit that referenced this pull request May 19, 2025
@nbianca
Use Nginx config outlets in templates (#959)
47506fd 
* Reapply "Use Nginx config with outlets (#913)" (#958)

This reverts commit 3c22348.

* Keep empty outlet files instead of removing them

This is necessary because `replace` operation checks if file exists
first. This is a fix for socketed template.

* Create placeholders for all templates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgxworld tgxworld tgxworld left review comments

@CvX CvX CvX approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nbianca @Firefishy @MichaIng @CvX @tgxworld